Skip to content

linux: the read-deny glob walk has a budget - #607

Open
ronleizrowice-ant wants to merge 14 commits into
mainfrom
fix/linux-read-deny-glob-bound
Open

ronleizrowice-ant wants to merge 14 commits into
mainfrom
fix/linux-read-deny-glob-bound

Conversation

@ronleizrowice-ant

@ronleizrowice-ant ronleizrowice-ant commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

What it does

On Linux the denyRead globs of one configuration are expanded on one shared budget: directory entries looked at, and wall time. When it runs out, wrapWithSandbox(), wrapWithSandboxArgv() and getFsReadConfig() fail with LinuxSandboxProfileError, code deny_glob_too_large. Nothing shortened is returned. SRT_DEBUG logs what each expansion cost.

The first commit's message describes a rule (stopping at links out of the tree) that the branch no longer has: links are followed as before.

Defaults and option

20,000,000 entries and 60,000 ms. filesystem.denyReadGlobBudget: { maxEntries?, timeoutMs? } (positive integers, no other key) sets others, from initialize(), updateConfig() or one wrap's customConfig. Only the schema checks the values, and like every other option they are not checked again on the last two routes. A monorepo of 2.5 million entries walks in about 7 s warm and fits.

Who sees a difference

Only a configuration whose patterns walk past the budget: it used to wait, now it is refused. Each ** pattern walks its tree again.

Known limits

initialize() and updateConfig() accept an over-budget configuration, and nothing is cached: every refused wrap spends the budget again, synchronously. One blocking filesystem call or one slow match is not cut short. All patterns share one deadline.

Merge order

Not independent: textual conflicts with #569 (README ~748, linux-sandbox-utils.ts ~753-758, sandbox-manager.ts imports), #573 (README ~442), #584 and #615 (README ~749, linux-sandbox-utils.ts ~763-770), #594 (linux-sandbox-utils.ts ~753-764), #575 (README ~442, sandbox-utils.ts ~1644-1662 and ~1723-1739, plus an unmarked break: listings.size is gone after #575, and records.size is no substitute: it also counts directories that could not be listed) and #608 (five hunks; a resolution that drops #608's anchor argument type-checks and silently loses it; anchored walks need the budget too). Whoever merges second resolves by hand.

bubblewrap has no patterns, so a denyRead entry with glob syntax is
expanded on the host into the paths to mount over, on every wrap. That
walk listed through any symbolic link to a directory, wherever it led;
the only link it declined was one leading back up the tree. With a link
from a project to /usr and `**/.env` denied, each expansion listed
57,000 entries: 260 to 320 ms per command warm, seconds cold, and no
limit for a larger target. A sandboxed command can plant such a link.

A link whose target lies outside the pattern's tree is no longer listed
through. The tree is the pattern's literal starting directory and what
is under it, and a link is judged by where it resolves. Links that stay
inside are followed as before, and a link whose own name matches still
masks what it leads to. What this gives up is a file the pattern matches
only by a name that passes through such a link: macOS never covered it,
since patterns are matched against resolved paths there.

Where a matched link hides a directory outside the tree that the pattern
would have carried on into, nothing is bound back beneath it: it is
reported through unlistableDenyDirs like a directory the walk cannot
list. Unlisted, an allowed path beneath it would otherwise come back
with nothing masked.

The walk takes a budget, shared by all the denyRead patterns of one
configuration: 2,000,000 directory entries and 10 seconds, the clock
read before each listing and at each entry. Out of budget it throws and
hands nothing back, and the wrap is refused with LinuxSandboxProfileError
`deny_glob_too_large`, the walk's error as its cause. getFsReadConfig()
throws the same. A deny list cut short would leave readable what the
pattern was written to hide.

Each expansion logs one line under SRT_DEBUG: milliseconds, matches,
mounts, directories listed, entries looked at, links left unfollowed.
Ten existing cases pinned the listing through a link out of the tree.
Five now expect the rule. Five are about something else (a chain of
links, a path too long to name, `**` written against text, a bracket
expression, a carve-out): they keep their expectations under a base
widened to hold the link, and assert the original pattern under the new
rule beside it. None is removed.

New cases pin the rule from both sides: a link out is never listed, by
a count of listings, and a link inside the tree that is the only route
to a match is still followed. A link is judged by where it resolves; the
tree is the pattern's and not the project's; a matched link still masks
its target. Nothing is bound back beneath an unlisted target, shown by a
real bubblewrap run in which the file beneath the allowed path is
unreadable under both names. The budget: the exact entry boundary, the
deadline before and during the walk and inside a small directory, one
budget shared by the patterns of a wrap, never a partial result. At the
manager the wrap rejects and returns no command, the getter throws the
same, and the fields of the cause are what a caller words its own
message from.
@ronleizrowice-ant ronleizrowice-ant mentioned this pull request Sep 25, 2026
…llowed

A link out of a denyRead pattern's tree is not listed through, and what
the pattern would have matched beneath it is not denied. That was
recorded on the walk and written to the debug log; a caller had no way
to tell its user. getFsReadConfig() now lists each such link in
`unfollowedDenyLinks`, with the pattern that came to it, the link and
where it resolves. It informs and restricts nothing: no backend reads
it. The type is exported as UnfollowedDenyLink.
It lists the links left unfollowed for leading out of a pattern's tree.
A link that leads back up the tree, and any link under a pattern that
cannot be followed one path component at a time, is not listed through
either and is not in the list. The field's comment and the README read
as if the list were of every link not followed.
Where a link whose own name matches hides a directory outside the
pattern's tree, the expansion reported that directory as one it could
not list, so that nothing was bound back beneath it. The wrapper then
restores no allowed path at or beneath it, the paths that are always
writable included: with a link to a directory above the command's
temporary directory, every command failed, and an allowed write path
beneath such a directory stopped applying. Through the ancestor that
hides it, a second link could take the working directory along.

The report is taken out again. A directory outside the tree is never
listed, whether or not the link that leads there matches and whatever
is allowed beneath it; a matched link still hides its target whole; an
allowed path beneath it is bound back as written, as beneath a directory
denied literally. `unlistableDenyDirs` again names only a directory the
walk could not list, and the code that fills it is what it was.

What that gives up is what such a link used to add: what the pattern
matched beneath the allowed path through the link's name is not masked
again. A link out of the tree can only add to what a pattern covers. A
file whose real path is in the tree and matches is found by walking the
tree itself, so no link a sandboxed command creates takes a mask away.

The unfollowed links are handed back sorted by name, so that the list
and the debug line do not depend on the order a directory is read in.
The cases written for the report of an unlisted directory now state the
rule as it is: what a matched link out of the tree leads to is hidden
whole and not listed, with nothing allowed beneath it and with an
allowed path at it, beneath it or written through the link; nothing is
handed back as unlistable; a link inside such a directory is never come
to. Two real bubblewrap runs pin what follows from it: beneath such a
directory an allowed path is readable and the rest stays hidden, under
either name, and a write allowed there succeeds. The unfollowed links
come back in the same order whichever way the directory is read.
The walk stopped at a symbolic link that leaves the pattern's tree.
Every release follows such a link and masks what it finds behind it:
with `project/out` a link to `../outside` and `project/**/.env` denied,
`outside/deep/.env` is unreadable under both names on 0.0.75, 0.0.76 and
0.0.77. Stopping there left it readable, so the rule lowered what a
pattern covers and is taken out. The messages of the earlier commits on
this branch that describe it as what other releases and platforms do
were wrong on that point.

Link handling in walkGlobPattern is what it was. `GlobWalk.unfollowedLinks`,
the out-parameter of expandReadDenyGlobLinux, and `unfollowedDenyLinks`
with its exported type are removed.

What stays is what the walk never had: a budget shared by all the
denyRead patterns of one configuration, 2,000,000 directory entries and
10 seconds, the clock read before each listing and at each entry. Out of
budget the walk throws and hands nothing back, the wrap is refused with
LinuxSandboxProfileError `deny_glob_too_large`, and getFsReadConfig()
throws the same. What lies behind a link counts against it like anything
else. Each expansion logs one line under SRT_DEBUG with what it cost.
…budget

The two existing glob suites are as they were before this branch. The
new file keeps the cases for the budget, for the refusal at the manager
and for the debug line, and loses the ones written for the rule. Added:
a match behind a link to a directory beside the project is denied where
it really is, and a real bubblewrap run shows it unreadable under both
names; what such a link leads to is spent from the budget, at the walk
and at the manager.
@ronleizrowice-ant ronleizrowice-ant changed the title linux: a read-deny glob stays in its own tree, and its walk has a budget linux: the read-deny glob walk has a budget Sep 26, 2026
Comments only: the JavaScript emitted with comments removed is
byte-identical for every file, and the test names are unchanged.

Kept: every invariant and what could be done without it, the
documentation of exported names, and the reasons for what is not done
the obvious way. Cut: how the code came to be as it is, narration of
the lines that follow, and explanations repeated at several sites.
…to change it

The default of 2,000,000 entries and 10 seconds refused every command
in a large monorepo: one `**/.env` pattern over 2.5 million entries
walks in about 7 seconds. Such a user waited before; refused, they
could not run anything, and nothing could raise the limit.

The defaults are now 20,000,000 entries and 60 seconds, and
`filesystem.denyReadGlobBudget` (`maxEntries`, `timeoutMs`) sets
others, from the initialized configuration, updateConfig() or one
wrap's customConfig. The refusal names the option.

README: wrapWithSandboxArgv() rejects the same way; the patterns share
one deadline; an over-budget configuration is accepted at initialize()
and every wrap of it spends the budget again.
A misspelt `maxEntires` was accepted and ignored, and the default
applied in silence.
ronleizrowice-ant added a commit that referenced this pull request Sep 30, 2026
Both it and the walk's budget (#607) hand every pattern of one read
configuration something to share, so the listings go where the budget already
is: made in readDenyGlobExpander(), and handed on in the options of
expandReadDenyGlobLinux(). The walk's record of a directory gives up its own
copy of the entries for the shared map.

An entry is paid for before it is skipped: the budget bounds the entries
looked at, and one the fast path passes over was looked at too.
ronleizrowice-ant added a commit that referenced this pull request Sep 30, 2026
The walk's budget (#607), its shared listings (#627) and its steps (#628)
rewrite one code path, and the path entries of #620 to #622 sit on top of it.

Everything above the walk is a generator now: the read-deny expansion, the
function readDenyGlobExpander() returns, the allowRead expansion, the
resolution of read path entries, and withOtherReadings(), which calls the
expander once for each reading of an entry. The synchronous names finish
those on the spot.

In the walk the step comes first and the budget's clock is read after it,
before the next listing. The deadline is wall time, so turns given to other
work count against it: a wrap can be refused earlier for that, and a list is
never cut short.

The budget error is given its code around the delegated generator. An abort
is raised by the driver and does not pass through there, so a wrap with a
spent budget and an aborted signal rejects with the signal's reason.

A wrap that starts over makes a new budget and new listings, and reads the
new configuration's denyReadGlobBudget.

In the scan's catch the abort is looked at first, then what ripgrep listed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant